Consolidate duplicate functions, fix fill_intent CEI, add struct fields, implement validate_proof - #436
Merged
james2177 merged 2 commits intoSep 30, 2026
Conversation
…ds, implement validate_proof ## Issue stellar-vortex-protocol#343: Collapse duplicate batch_fill_intent, batch_cancel_intent, get_pending_admin Removed duplicate function definitions that differed in authorization strategy. The correct implementations authorize once per batch at the beginning (require_auth() once), avoiding Soroban's restriction on multiple require_auth() calls for the same address in one invocation. Consolidated to single CEI-correct versions in each case: batch_fill_intent() calls require_auth() once then fill_intent_inner() for each fill; batch_cancel_intent() calls require_auth() once with unified cooldown stamping; get_pending_admin() read-only accessor returns pending admin tuple. Deleted incorrect duplicates that called require_auth per-item or per-call, which would panic at runtime. ## Issue stellar-vortex-protocol#344: Fix fill_intent transferring output twice with undefined fee variable Removed the premature DST token transfer at line 2252 (before CEI) that violated the carefully-sequenced state-first pattern the contract relies on for reentrancy protection. Removed the undefined protocol_fee_bps variable calculation at line 2255 that would not compile. Now the function follows strict CEI ordering: all storage writes (intent state, solver record, volume totals) happen before any external calls. The single transfer to user + fee splits to recipient/referrer both execute at the end via the properly-computed fee from get_tiered_fee_bps(), guaranteeing atomicity and reentrancy safety. ## Issue stellar-vortex-protocol#345: Add missing IntentRecord and ProtocolConfig fields for referral and slash-cycle features Added referrer: Option<Address> to IntentRecord to support the referral fee-share logic in stellar-vortex-protocol#281. Added slash_cycles: u32 to track slash count and enforce the per-solver bound. Added referral_share_bps: i128 to ProtocolConfig to control the percentage of fees split to referrers (capped at 10_000 bps = 100%). Added max_slash_cycles: u32 to ProtocolConfig to set the forced-deregistration limit per solver. These fields were referenced in fee-split and slash-rate code but did not exist on the structs, causing compilation failures for merged features stellar-vortex-protocol#281 and stellar-vortex-protocol#241. ## Issue stellar-vortex-protocol#346: Replace no-op validate_proof stub with real implementation Deleted the second validate_proof definition that did nothing but fetch the registry address and return ("In production, this would..."), leaving the first implementation as the single source of truth. The remaining validate_proof fetches ProofRegistryClient::get_fresh_proof, validates src_chain_id against the intent via wormhole_chain_id(), and checks src_amount >= intent.src_amount. Proof-gated fills now actually verify proof.src_chain and proof.src_amount, closing the gap where require_proof=true silently gated on nothing. Closes stellar-vortex-protocol#343 Closes stellar-vortex-protocol#344 Closes stellar-vortex-protocol#345 Closes stellar-vortex-protocol#346
|
@thelux134 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four high-priority fixes to the Vortex Protocol's settlement contract, resolving duplicate function definitions, correcting CEI-order violations, adding missing struct fields for merged features, and replacing a no-op proof-validation stub.
Issue #343: Collapse duplicate batch_fill_intent, batch_cancel_intent, get_pending_admin
Removed duplicate function definitions. The duplicates differed only in authorization strategy — one pair called require_auth() per-item (which Soroban forbids), while the other called it once per batch (correct). Consolidated to the CEI-correct single versions:
batch_fill_intent(): Authorizes solver once at entry, then callsfill_intent_inner()for each fillbatch_cancel_intent(): Authorizes user once, checks cooldown once, stamps cooldown once, atomically processes all cancelsget_pending_admin(): Single read-only accessorDeleted the incorrect duplicates. Added a guard test that parses the exported contract spec and asserts no duplicate function names exist.
Issue #344: Fix fill_intent transferring output twice with undefined fee
Removed the premature DST token transfer that violated CEI ordering (line 2252, before state writes). Removed the incorrect fee calculation using undefined
protocol_fee_bpsvariable (line 2255). Now the function follows strict CEI:The fee is computed once via
get_tiered_fee_bps()with checked arithmetic, honoring volume discounts (#192), and split with referrer when applicable.Issue #345: Add missing IntentRecord and ProtocolConfig fields
Added four fields required by merged feature branches:
IntentRecord.referrer: Option<Address>— referral fee-share recipient ([High] Implement an optional on-chain referral fee-share forsubmit_intent#281)IntentRecord.slash_cycles: u32— slash count toward forced deregistration ([High] Add a cap on repeatedOpen → Accepted → Slashedcycles per intent #241)ProtocolConfig.referral_share_bps: i128— fraction of fees (0–10,000 bps) sent to referrerProtocolConfig.max_slash_cycles: u32— solver deregistration thresholdThese fields were referenced in fee-split and slash-rate code but did not exist, causing compilation failures.
Issue #346: Replace no-op validate_proof stub
Deleted the second
validate_proofdefinition that did nothing ("In production, this would..."). The remaining implementation now actually verifies:ProofRegistryClient::get_fresh_prooffor the intentproof.src_chain_id == src_chain_to_wormhole_id(intent.src_chain)proof.src_amount >= intent.src_amountProof-gated fills (
require_proof=true) now actually verify the proof instead of silently returning success.Test Plan
batch_fill_intent,batch_cancel_intent,get_pending_adminin exported specfill_intent_innerwith referrer and without (fee split correct)validate_proofrejects mismatched src_chain, insufficient src_amounttransfer()is rejected post-CEI state commitcloses [High] Collapse the duplicated
batch_fill_intent,batch_cancel_intent, andget_pending_admindefinitions #343closes [High] Add the missing
IntentRecordandProtocolConfigfields (referrer,slash_cycles,referral_share_bps,max_slash_cycles) #345closes [High] Fix
fill_intenttransferring the user's output twice and computing the fee from an undefined variable #344closes [High] Replace the no-op
validate_proofstub sorequire_proof = trueactually verifies the source deposit #346